salt: add x509 extensions 'subjectKeyIdentifier' and 'authorityKeyIdentifier' to certs by ezekiel-alexrod · Pull Request #4836 · scality/metalk8s

ezekiel-alexrod · 2026-03-24T17:14:52Z

Add x509 extensions to CA and leaf certificates (RFC 5280)

Summary

Add subjectKeyIdentifier (SKI) to all 6 CA certificates and authorityKeyIdentifier (AKI) to all leaf certificates, per RFC 5280.

Context

The certificates generated by MetalK8s were missing standard x509 extensions required by RFC 5280:

Extension	CA self-signed	Leaf certs	Reference
`subjectKeyIdentifier: hash`	MUST	not included (SHOULD)	Section 4.2.1.2
`authorityKeyIdentifier: keyid`	MAY be omitted	MUST	Section 4.2.1.1

Per Section 4.2.1.2, the SKI value on the CA is the value that gets copied into the AKI keyid field of certificates issued by that CA, establishing the identifier chain: "the value of the subject key identifier MUST be the value placed in the key identifier field of the authority key identifier extension of certificates issued by the subject of this certificate".

Changes

CA certificates (6 files) -- added subjectKeyIdentifier: hash to x509.certificate_managed:

salt/metalk8s/kubernetes/ca/kubernetes/installed.sls
salt/metalk8s/kubernetes/ca/etcd/installed.sls
salt/metalk8s/kubernetes/ca/front-proxy/installed.sls
salt/metalk8s/addons/dex/ca/installed.sls
salt/metalk8s/addons/nginx-ingress/ca/installed.sls
salt/metalk8s/backup/certs/ca.sls

Signing policies (1 file) -- added authorityKeyIdentifier: keyid to all 8 policies in:

pillar/metalk8s/roles/ca.sls

Leaf certificate states (13 files) -- added authorityKeyIdentifier: keyid to each x509.certificate_managed state. This is needed because the signing policies alone control what extensions are stamped on new certs, but Salt does not compare signing policy extensions against existing certs to trigger re-issuance. Adding the extension in the state enables Salt to detect the diff during upgrade.

salt/metalk8s/kubernetes/apiserver/certs/{server,kubelet-client,front-proxy-client,etcd-client}.sls
salt/metalk8s/kubernetes/etcd/certs/{server,peer,healthcheck-client}.sls
salt/metalk8s/salt/master/certs/{salt-api,etcd-client}.sls
salt/metalk8s/addons/dex/certs/server.sls
salt/metalk8s/addons/nginx-ingress/certs/server.sls
salt/metalk8s/addons/nginx-ingress-control-plane/certs/server.sls
salt/metalk8s/backup/certs/server.sls

Test data -- updated salt/tests/unit/formulas/data/base_pillar.yaml with matching signing policy changes.

Upgrade behavior

On upgrade from 132.0.2, Salt detects Certificate properties are different: X509v3 Extensions and re-issues each certificate while preserving the existing private keys. The upgrade orchestration handles this through bootstrap/pre-upgrade.sls (CA certs) and the per-node highstate (leaf certs).

Test results

Tested manually on fresh EC2 instances (Rocky 8.9, 3 nodes):

Fresh install 133.0.0-dev (3 nodes):

Check	Result
3 nodes Ready (v1.33.7)	OK
6 CA certs: SKI present, no AKI	6/6 OK
8 leaf certs bootstrap: AKI present	8/8 OK
6 leaf certs workers: AKI present	6/6 OK
AKI keyid == CA SKI	match

Upgrade 132.0.2 -> 133.0.0-dev (3 nodes):

Check	Pre-upgrade (132.0.2)	Post-upgrade (133.0.0-dev)
3 nodes Ready	v1.32.7	v1.33.7
6 CA certs: SKI	absent	present
6 CA certs: AKI	absent	absent (MAY omit per RFC)
8 leaf certs bootstrap: AKI	absent	present
6 leaf certs workers: AKI	absent	present
AKI keyid == CA SKI	N/A	match
CA private keys	fingerprint A	fingerprint A (unchanged)

Single-node upgrade 132.0.2 -> 133.0.0-dev:

Check	Result
Bootstrap Ready (v1.33.7)	OK
6 CA certs: SKI added	6/6 OK
8 leaf certs: AKI added	8/8 OK
AKI keyid == CA SKI	match
CA private keys unchanged	OK

Idempotent re-deploy (highstate on existing 133.0.0-dev):

Check	Result
CA cert fingerprints unchanged	OK (no unnecessary re-generation)
CA key fingerprints unchanged	OK
All pods Running	OK

Test plan

Fresh install 3 nodes: 6 CAs have SKI, 14 leaf certs have AKI, keyid matches
Upgrade 132.0.2 -> 133.0.0-dev (3 nodes): extensions added, CA keys unchanged, cluster healthy
AKI keyid in leaf certs matches SKI of issuing CA
CI nightly: no regressions (same failures as development/133.0)

Closes: MK8S-201

bert-e · 2026-03-24T17:14:56Z

Hello ezekiel-alexrod,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-03-24T17:15:03Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

claude · 2026-03-24T17:15:48Z

LGTM

Clean, consistent change across all 6 CA certificate states. The extensions follow RFC 5280 recommendations, the CHANGELOG entry is correctly placed, and the upgrade safety testing is thorough.

Review by Claude Code

salt/metalk8s/addons/dex/ca/installed.sls

bert-e · 2026-03-25T08:22:42Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

Add `subjectKeyIdentifier` (SKI) to all 6 CA certificates and `authorityKeyIdentifier` (AKI) to all leaf certificates via the 8 x509 signing policies, per RFC 5280: - SKI on CA certs: MUST (Section 4.2.1.2) - AKI on leaf certs: MUST (Section 4.2.1.1) - AKI on self-signed CAs: MAY be omitted (Section 4.2.1.1) The AKI keyid in leaf certs references the SKI of the issuing CA, establishing the chain of trust identifier required by the RFC. On upgrade, Salt detects the missing extensions and re-issues the certificates while preserving the existing private keys. Closes: MK8S-201

claude · 2026-03-25T15:49:36Z

LGTM

All 6 CA certificates correctly have subjectKeyIdentifier: hash added, all 8 signing policies have authorityKeyIdentifier: keyid added, and the test pillar data is updated to match. The changes are consistent and aligned with RFC 5280 requirements.

Review by Claude Code

Add `authorityKeyIdentifier: keyid` to all 13 leaf certificate `x509.certificate_managed` states so that Salt detects the missing extension on existing certs and re-issues them during upgrade. The signing policies alone (in pillar) control what extensions are stamped on new certs, but Salt does not compare signing policy extensions against existing certs. Adding the extension in the state itself triggers the diff detection needed for upgrade. Closes: MK8S-201

bert-e · 2026-03-26T18:22:37Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

claude · 2026-03-26T18:24:06Z

salt/metalk8s/addons/dex/certs/server.sls

    - signing_policy: {{ dex.cert.server_signing_policy }}
    - CN: dex-server
    - subjectAltName: "{{ salt['metalk8s.format_san'](certSANs | unique) }}"
+    - authorityKeyIdentifier: keyid


authorityKeyIdentifier: keyid is now specified in both the signing policy (pillar) and in each individual leaf cert state file. The signing policy already enforces this extension on all certificates it signs, so the per-cert declarations are redundant.

This duplication means if the extension needs to change in the future, it must be updated in 8 signing policies + 13 individual state files instead of just the 8 policies. Consider removing the authorityKeyIdentifier lines from the individual leaf cert state files and relying solely on the signing policies in pillar/metalk8s/roles/ca.sls.

— Claude Code

The duplication is intentional and required for upgrade correctness.

We tested with authorityKeyIdentifier: keyid only in the signing policies (without it in the individual leaf cert states). The result: on upgrade from 132.0.2, Salt did not re-issue any leaf cert -- all leaf certs remained without AKI post-upgrade.

The reason is that the signing policy controls what extensions are stamped when a cert is issued, but x509.certificate_managed only compares properties declared in the state itself to decide whether to re-issue an existing cert. Since the signing policy lives on the CA side (pillar), the minion-side state has no visibility into it for diff detection.

So both are needed:

The signing policy ensures new certs (fresh install) get the AKI

The state declaration ensures Salt detects the missing AKI on existing certs and triggers re-issuance during upgrade

This was verified on 3-node EC2 clusters: without the per-cert declarations, AKI was present on fresh install but absent after upgrade. With both, it works in all scenarios.

claude · 2026-03-26T18:24:11Z

authorityKeyIdentifier: keyid is duplicated between signing policies (pillar/metalk8s/roles/ca.sls) and each individual leaf cert state file (13 files). The signing policy should be sufficient — consider removing the per-cert declarations to avoid dual maintenance.

Review by Claude Code

bert-e · 2026-03-26T20:39:53Z

Conflict

There is a conflict between your branch improvement/add-x509-extensions-CA-certs and the
destination branch development/133.0.

Please resolve the conflict on the feature branch (improvement/add-x509-extensions-CA-certs).

git fetch && \
git checkout origin/improvement/add-x509-extensions-CA-certs && \
git merge origin/development/133.0

Resolve merge conflicts and commit

git push origin HEAD:improvement/add-x509-extensions-CA-certs

ezekiel-alexrod requested a review from a team as a code owner March 24, 2026 17:14

TeddyAndrieux reviewed Mar 25, 2026

View reviewed changes

salt/metalk8s/addons/dex/ca/installed.sls Outdated Show resolved Hide resolved

ezekiel-alexrod force-pushed the improvement/add-x509-extensions-CA-certs branch from 081d828 to 5d0dbef Compare March 25, 2026 15:48

TeddyAndrieux previously approved these changes Mar 26, 2026

View reviewed changes

ezekiel-alexrod dismissed TeddyAndrieux’s stale review via 9f38ed7 March 26, 2026 18:22

claude bot reviewed Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

salt: add x509 extensions 'subjectKeyIdentifier' and 'authorityKeyIdentifier' to certs#4836

salt: add x509 extensions 'subjectKeyIdentifier' and 'authorityKeyIdentifier' to certs#4836
ezekiel-alexrod wants to merge 2 commits intodevelopment/133.0from
improvement/add-x509-extensions-CA-certs

ezekiel-alexrod commented Mar 24, 2026 •

edited

Loading

Uh oh!

bert-e commented Mar 24, 2026

Uh oh!

bert-e commented Mar 24, 2026

Uh oh!

claude bot commented Mar 24, 2026

Uh oh!

Uh oh!

bert-e commented Mar 25, 2026

Uh oh!

claude bot commented Mar 25, 2026

Uh oh!

bert-e commented Mar 26, 2026

Uh oh!

claude bot Mar 26, 2026

Uh oh!

ezekiel-alexrod Mar 26, 2026

Uh oh!

claude bot commented Mar 26, 2026

Uh oh!

bert-e commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ezekiel-alexrod commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add x509 extensions to CA and leaf certificates (RFC 5280)

Summary

Context

Changes

Upgrade behavior

Test results

Test plan

Uh oh!

bert-e commented Mar 24, 2026

Hello ezekiel-alexrod,

Uh oh!

bert-e commented Mar 24, 2026

Waiting for approval

Uh oh!

claude bot commented Mar 24, 2026

Uh oh!

Uh oh!

bert-e commented Mar 25, 2026

Waiting for approval

Uh oh!

claude bot commented Mar 25, 2026

Uh oh!

bert-e commented Mar 26, 2026

Waiting for approval

Uh oh!

claude bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

ezekiel-alexrod Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot commented Mar 26, 2026

Uh oh!

bert-e commented Mar 26, 2026

Conflict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ezekiel-alexrod commented Mar 24, 2026 •

edited

Loading